Skip to content

feat(k8s-reporter): add extraVolumes, extraVolumeMounts, extraEnvVars, customCA#777

Merged
AlexKantor87 merged 8 commits intomainfrom
k8s-reporter-extra-volumes-and-custom-ca
Apr 15, 2026
Merged

feat(k8s-reporter): add extraVolumes, extraVolumeMounts, extraEnvVars, customCA#777
AlexKantor87 merged 8 commits intomainfrom
k8s-reporter-extra-volumes-and-custom-ca

Conversation

@AlexKantor87
Copy link
Copy Markdown
Contributor

Closes #776.

Summary

Adds two layers of support for customers running the k8s-reporter behind a TLS-inspecting proxy that requires a corporate CA bundle:

  1. Generic escape hatchextraVolumes, extraVolumeMounts, extraEnvVars (Bitnami / prometheus-community naming convention).
  2. customCA convenience wrapper — single config block that mounts the CA via subPath into /etc/ssl/certs/ so Go's stdlib picks it up additively alongside the system bundle. Deliberately does not set SSL_CERT_FILE — that would replace the system bundle and break trust for any public CAs the customer's bundle does not include.

No changes to the Kosli CLI itself: it's Go and uses net/http's default transport, so crypto/x509 reads /etc/ssl/certs/ automatically.

Files changed

  • charts/k8s-reporter/values.yaml — new top-level fields with documented examples
  • charts/k8s-reporter/templates/cronjob.yaml — three injection points (volumes, volumeMounts, env)
  • charts/k8s-reporter/Chart.yaml — bump 2.1.02.2.0 (additive, minor)
  • charts/k8s-reporter/_templates.gotmpl — new "Running behind a TLS-inspecting proxy" docs section
  • charts/k8s-reporter/README.md.gotmpl — wire the new section into the README
  • charts/k8s-reporter/README.md + docs.kosli.com/content/helm/_index.md — regenerated via make helm-docs

Verification

helm lint and helm template validated against five scenarios:

Scenario Result
Baseline (both layers off) identical to main — no behaviour change for existing users
customCA only volume + subPath mount injected at correct nesting
Generic fields only extraVolumes, extraVolumeMounts, extraEnvVars all render
Both layers together both inject without collision
customCA.enabled=true with no secretName clean error: customCA.secretName is required when customCA.enabled is true

make helm-lint passes (strict mode too).

Test plan

  • Reviewer renders helm template locally with customCA.enabled=true and confirms the mount appears at /etc/ssl/certs/kosli-custom-ca.crt with subPath: ca.crt
  • Reviewer renders helm template with the generic extraVolumes example from values.yaml and confirms it appears alongside the chart's own environments-config volume
  • CI: make helm-lint passes
  • CI: make helm-docs produces no diff (already regenerated in this PR)

Deliberately deferred

  • Helm template golden-file tests in CI. The chart has no test infrastructure today (no tests/ directory, CI only runs helm lint). Adding a snapshot/golden-file harness is a meaningful new pattern for this repo and belongs as its own thin slice rather than bundled here. Suggest a follow-up issue if desired.
  • customCA.mountPath / volume-name override. The wrapper hardcodes /etc/ssl/certs/kosli-custom-ca.crt and volume name custom-ca to keep the UX one-decision-deep. Customers who need different conventions should use the generic fields.

Customer context

Reported by a customer whose environment sits behind a TLS-inspecting appliance that re-signs HTTPS traffic with a corporate CA. They confirmed that the dedicated customCA wrapper would materially help their deployment flow, which is why it ships in the same PR as the generic fields rather than as a follow-up.

…, customCA

Adds two layers of support for customers running the k8s-reporter behind
a TLS-inspecting proxy that requires a corporate CA bundle:

1. Generic escape hatch — extraVolumes, extraVolumeMounts, extraEnvVars
   following Bitnami / prometheus-community naming conventions.
2. customCA convenience wrapper — single config block that mounts the
   CA via subPath into /etc/ssl/certs/ so Go's stdlib picks it up
   additively alongside the system bundle (avoids the SSL_CERT_FILE
   footgun that would otherwise replace the system CAs).

The Kosli CLI is Go and uses net/http's default transport; no CLI
changes are needed because crypto/x509 reads /etc/ssl/certs/ automatically.

Refs #776

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 15, 2026

Claude finished @AlexKantor87's task in 3m 16s —— View job


PR Review: feat(k8s-reporter): add extraVolumes, extraVolumeMounts, extraEnvVars, customCA

  • Run static analysis checks (go build, go vet, go mod tidy)
  • Read and review all changed files
  • Post inline comments on specific issues
  • Post summary review

Static Analysis

All passing — no issues:

  • go build ./... — clean
  • go vet ./... — clean
  • go mod tidy — no diff in go.mod/go.sum

(Could not run helm lint / helm template due to tool permissions — recommend verifying in CI.)


Review Summary

This is a well-structured, additive-only Helm chart change. The previous two review rounds caught real issues (required guard on customCA.key, env value quoting, whitespace trimming) and all have been addressed. The current state looks solid.

What looks good

  • Correct subPath approach: Mounting the CA cert as a single file via subPath into /etc/ssl/certs/ is the right call — Go's crypto/x509 directory scan picks it up additively without overriding the system bundle. The deliberate decision not to set SSL_CERT_FILE avoids a common footgun.
  • required guards on both secretName and key: Fail-fast with clear error messages rather than producing broken manifests.
  • Quoting fixes: Adding | quote to podLabels values, env values, secretName, and subPath protects against YAML-special characters. Good defensive improvement that also fixes pre-existing issues.
  • Whitespace trimming: {{- end }} on podLabels and env ranges cleans up blank lines in rendered output.
  • nindent values are correct: 10 for volumes, 12 for volumeMounts, 14 for env entries — all match the existing indentation structure.
  • README and docs in sync: charts/k8s-reporter/README.md and docs.kosli.com/content/helm/_index.md are identical (verified via diff).
  • Thorough documentation: The _templates.gotmpl section explains why the design works (Go cert loading behavior, PSS compatibility, SSL_CERT_FILE trade-off) — not just how to use it.
  • Chart version bump: 2.1.0 → 2.2.0 is correct semver for additive features.

Minor observations (non-blocking)

  1. Pre-existing: dryRun/httpProxy arg blocks (cronjob.yaml:94-100) — these still use {{ if }}/{{ end }} without leading hyphens, producing extra blank lines in rendered YAML. Now the only remaining instances after this PR's cleanup. Worth a follow-up one-liner.

  2. defaultMode: 0644 — correct (YAML 1.1 interprets leading 0 as octal → decimal 420 = rw-r--r--), but worth noting that some linters flag bare octal literals. If a future linter run catches this, the fix is just wrapping in quotes: defaultMode: "0644".

Verdict

LGTM — clean, well-documented, and the previous review feedback has been thoroughly addressed. The only remaining nit is the pre-existing whitespace issue in the dryRun/httpProxy blocks, which is outside this PR's scope.

Comment thread charts/k8s-reporter/templates/cronjob.yaml Outdated
Comment thread charts/k8s-reporter/templates/cronjob.yaml Outdated
AlexKantor87 and others added 2 commits April 15, 2026 10:19
…recation

Addresses parallel review feedback on PR #777:

- Add explicit defaultMode: 0644 to customCA Secret volume so the
  permission posture is self-documenting for auditors.
- Tighten the docs description of Go's cert-pool loading: it's two
  independent passes (cert file + dir scan), not one unified scan.
- Add the literal "x509: certificate signed by unknown authority"
  string to the docs section so customers searching the error message
  find it.
- Replace the SSL_CERT_FILE example in the extraEnvVars block with
  HTTPS_PROXY — SSL_CERT_FILE is the documented footgun and shouldn't
  be the first thing copy-pasters see.
- Shorten the customCA parent comment so the helm-docs values table
  reads cleanly; defer detail to the README section.
- Mark the existing `env:` map as DEPRECATED in favour of extraEnvVars
  to remove cross-reference confusion. Both still work.

Refs #776

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Backing out the env-map deprecation from the previous commit — that was
scope creep beyond the customCA feature, made without auditing who
relies on env: today or whether the team wants to phase it out.

Instead: surface env: in the helm-docs values table (it was previously
invisible because the values.yaml entry was fully commented out) and
describe env: vs extraEnvVars as sibling options. Behaviour unchanged
— env: {} is functionally identical to the previous fully-commented form.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread charts/k8s-reporter/templates/cronjob.yaml Outdated
Comment thread charts/k8s-reporter/templates/cronjob.yaml Outdated
Comment thread charts/k8s-reporter/values.yaml
AlexKantor87 and others added 2 commits April 15, 2026 10:26
- Add `required` guard on customCA.key to prevent the silent
  empty-subPath footgun (an empty subPath causes K8s to mount the
  whole Secret as a directory at the mountPath, which would break Go
  trying to read it as a cert file).
- Tighten the env range loop with `{{- end }}` so the rendered env
  block doesn't carry a trailing blank line before extraEnvVars.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reverting the editorial framing in the env: comment. The original
comment was a directive for single-tenant Kosli instances, not an
example. Also dropping the extraEnvVars cross-reference — that was
scope creep added without justification.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread charts/k8s-reporter/values.yaml Outdated
Comment thread charts/k8s-reporter/templates/cronjob.yaml
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Comment thread charts/k8s-reporter/values.yaml Outdated
Comment thread charts/k8s-reporter/values.yaml
Comment thread charts/k8s-reporter/templates/cronjob.yaml Outdated
Picking up safe-to-fix nits regardless of strict scope:

- Quote customCA.secretName and subPath, matching the pattern used for
  cronSchedule and image fields. mbevc1 also asked for this.
- Quote $value in the env range so user-supplied values containing
  YAML special chars (colons, brackets) render safely.
- Tighten the podLabels range with `{{- end }}` for consistency with
  the env-range fix in commit b44df21. Pre-existing whitespace nit,
  not introduced by this PR, but cheap to fix while I'm here.
- Collapse the env: helm-docs comment to a single line so it reads
  cleanly in the rendered values table (the multi-line YAML example
  collapsed badly after helm-docs concatenation).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread charts/k8s-reporter/templates/cronjob.yaml Outdated
Comment thread charts/k8s-reporter/templates/cronjob.yaml
Same fix as the env value quoting in f2af5bf, applied to podLabels
for consistency. Also fixes the double-space typo (`:=  .Values.`)
in both range loops — harmless cosmetic carry-over from the original
template.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@AlexKantor87 AlexKantor87 enabled auto-merge (squash) April 15, 2026 10:24
@AlexKantor87 AlexKantor87 merged commit 4c47ef2 into main Apr 15, 2026
20 checks passed
@AlexKantor87 AlexKantor87 deleted the k8s-reporter-extra-volumes-and-custom-ca branch April 15, 2026 10:41
@tooky
Copy link
Copy Markdown
Contributor

tooky commented Apr 15, 2026

Do we need to pull docs changes through to mintlify?

@mbevc1
Copy link
Copy Markdown
Contributor

mbevc1 commented Apr 15, 2026

There is a scheduled run that should pick this up

@mbevc1
Copy link
Copy Markdown
Contributor

mbevc1 commented Apr 15, 2026

@tooky ignore previous comment, it needed manual prompt. Automation is there only for backed and CLI + TF provider

@mbevc1
Copy link
Copy Markdown
Contributor

mbevc1 commented Apr 15, 2026

kosli-dev/docs#148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

charts/k8s-reporter: add extraVolumes, extraVolumeMounts, extraEnvVars, and customCA for corporate CA bundle support

3 participants